feat: Charge channel opening fee through lsp flow #1528

holzeis · 2023-10-26T09:03:19Z

This change was fairly simple with the new channel config flag accept_underpaying_htlcs 🎉. The main change is in this commit abb2feb

open channel fee + funding transaction is now shown on the payment which triggered the inbound channel creation
cleanup of all occurrences where we were using the order matching fee or jit channel fee invoice

resolves #843
fixes #1147
fixes #1366
fixes #1445

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-10-26.at.08.48.12.mp4

Unrelated minor fixes that could get separated into a single PRs.

c89154c: The app does not need to maintain a pending intercepted htlc map.
114fd52: We have to fail the payment backwards in case we do not get a pre-image
643b329: Add an argument to optionally do not truncate the provided value. fixes Pnl is not shown fully #1521
82a712f: Fix an invalid sql file

bonomat

Awesome, looking forward to having this in production :)

crates/ln-dlc-node/src/ln/app_event_handler.rs

crates/ln-dlc-node/src/config.rs

crates/ln-dlc-node/src/ln/coordinator_event_handler.rs

bonomat · 2023-10-26T11:24:17Z

crates/ln-dlc-node/src/ln/app_event_handler.rs

+                            format!("Couldn't find channel for user_channel_id {user_channel_id}",)
+                        })?;
+
+                    if channel.channel_state == ChannelState::Open {


❓
I don't understand, why would you fail if the channel is already open?
Wouldn't that prevent you from receiving payments?

Because then it is already been paid for.

ChannelReady -> OpenUnpaid
PaymentClaimable -> Open

I still don't understand: isn't the event PaymentClaimable called for all payments? Meaning, even after the channel was opened paid successfully, if the user tries to receive something, we would fail here?

Only if counterparty_skimmed_fee_msat > 0, or in other words only if the counterparty is skimming fees from the payment.

I guess this scenario means that the coordinator is trying to take extra fees from random payments after the JIT channel has already been created and the opening fee has been paid.

So the app should reject any payment that is taking extra fees arbitrarily. Perhaps eventually it would make sense to show this to the user, so that they can decide whether they want to pay for the special fee or not.

bonomat · 2023-10-26T11:34:18Z

Unrelated minor fixes that could get separated into a single PRs.

* [43d4999](https://github.com/get10101/10101/commit/43d49997f0b31ac2958398d7bf90492e8d365c0c): The app does not need to maintain a pending intercepted htlc map.

* [97f8db4](https://github.com/get10101/10101/commit/97f8db4ad61bcd94b09a86eff71e97467510d8d3): We have to fail the payment backwards in case we do not get a pre-image

* [4af7474](https://github.com/get10101/10101/commit/4af7474602044b343783d7dd49689f0ad722085e): Add an argument to optionally do not truncate the provided value. fixes

You might want to top getting some of the fixes in separate PRs straight into main and not into the 116-upgrade. I assume 116 might take a bit longer to land.

luckysori

LGTM.

coordinator/migrations/2023-10-25-120015_add_fee_to_channel/down.sql

coordinator/src/node/storage.rs

crates/ln-dlc-node/src/config.rs

luckysori · 2023-10-31T02:27:08Z

crates/ln-dlc-node/src/lib.rs

+    /// If the payment was used to open an inbound channel, this tx id refers the funding
+    /// transaction for opening the channel.
+    pub funding_txid: Option<Txid>,


🙃 Perhaps not worth it here, but it's nice to think about ways to avoid optional fields.

For instance, we could extend the PaymentFlow enum to something like:

enum PaymentType { Inbound, Outbound, Funding { funding_txid: Txid, } }

I like it 👍

opted to not do the change as it became too big of a change very quickly :/

crates/ln-dlc-node/src/ln/app_event_handler.rs

luckysori · 2023-10-31T02:42:55Z

crates/ln-dlc-node/src/tests/just_in_time_channel/channel_close.rs

@@ -33,10 +33,12 @@ async fn ln_collab_close() {
    .await
    .unwrap();

+    let fee_sats = 10_000;


🔧 Should this not come from the liquidity options or something like that?

technically you are right, but on these tests we do not know about the liquidity options. This is only known on the application layer of the coordinator and the app. I am using a little hack to inject the fees in the liquidity request, but for that I have to do it manually.

see also https://github.com/get10101/10101/pull/1528/files#diff-a6f251cebbb64254ec28d79e916edc0275557625800fae9588e2cdc2598a545dR270-L276

mobile/native/migrations/2023-10-25-120015_add_fee_to_channel/down.sql

IF EXISTS does not exist on sql lite. I stumbled upon that issue when trying to rerun all sql migration scripts on the app.

holzeis requested review from bonomat, Restioson and luckysori October 26, 2023 09:03

holzeis self-assigned this Oct 26, 2023

This was linked to issues Oct 26, 2023

Pnl is not shown fully #1521

Closed

JIT - Channel Fee: LSP Flow #843

Closed

Invoice descriptions should not be trusted #1147

Closed

holzeis force-pushed the feat/jit-channel-fee-lsp-flow branch 3 times, most recently from e310274 to c684b7d Compare October 26, 2023 10:17

bonomat approved these changes Oct 26, 2023

View reviewed changes

holzeis linked an issue Oct 26, 2023 that may be closed by this pull request

flaky tests::just_in_time_channel::create::open_jit_channel failed without payed jit channel opening fees on CI #1366

Closed

holzeis mentioned this pull request Oct 26, 2023

Funded wallet without paying the jit channel opening fees #1344

Closed

holzeis force-pushed the feat/jit-channel-fee-lsp-flow branch from c684b7d to 82a712f Compare October 27, 2023 06:26

holzeis linked an issue Oct 27, 2023 that may be closed by this pull request

Route not found when paying JIT fee #1445

Closed

luckysori approved these changes Oct 31, 2023

View reviewed changes

luckysori force-pushed the chore/upgrade-to-rust-lightning-116 branch 3 times, most recently from aa6fe70 to 32a9226 Compare November 1, 2023 15:00

holzeis force-pushed the feat/jit-channel-fee-lsp-flow branch 3 times, most recently from 9d0c54b to 2e19e87 Compare November 3, 2023 06:47

luckysori force-pushed the chore/upgrade-to-rust-lightning-116 branch from 32a9226 to 8ef9415 Compare November 3, 2023 10:23

holzeis force-pushed the feat/jit-channel-fee-lsp-flow branch from 2e19e87 to dd8812b Compare November 3, 2023 10:24

luckysori force-pushed the chore/upgrade-to-rust-lightning-116 branch from 8ef9415 to 247a892 Compare November 3, 2023 11:07

holzeis force-pushed the feat/jit-channel-fee-lsp-flow branch from dd8812b to 82d89a3 Compare November 3, 2023 11:08

Base automatically changed from chore/upgrade-to-rust-lightning-116 to main November 3, 2023 11:57

holzeis added 2 commits November 3, 2023 14:02

chore: Remove pending intercepted htlcs from app event handler

1c2d919

fix: Fail htlc backwards in case of missing preimage

c0021e7

holzeis added 3 commits November 3, 2023 14:02

fix: Add truncate argument to history detail

1aa7724

feat: Charge channel opening fee through the lsp flow

f956de1

fix: Remove invalid sql expression

f71d546

IF EXISTS does not exist on sql lite. I stumbled upon that issue when trying to rerun all sql migration scripts on the app.

holzeis force-pushed the feat/jit-channel-fee-lsp-flow branch from 82d89a3 to f71d546 Compare November 3, 2023 13:02

holzeis enabled auto-merge November 3, 2023 13:03

holzeis added this pull request to the merge queue Nov 3, 2023

Merged via the queue into main with commit ce33f34 Nov 3, 2023
8 checks passed

holzeis deleted the feat/jit-channel-fee-lsp-flow branch November 3, 2023 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Charge channel opening fee through lsp flow #1528

feat: Charge channel opening fee through lsp flow #1528

holzeis commented Oct 26, 2023 •

edited

Loading

bonomat left a comment

bonomat Oct 26, 2023

holzeis Oct 27, 2023

bonomat Oct 28, 2023

holzeis Oct 28, 2023

luckysori Oct 31, 2023

bonomat commented Oct 26, 2023

luckysori left a comment

luckysori Oct 31, 2023

holzeis Nov 2, 2023

holzeis Nov 2, 2023

luckysori Oct 31, 2023

holzeis Nov 2, 2023

feat: Charge channel opening fee through lsp flow #1528

feat: Charge channel opening fee through lsp flow #1528

Conversation

holzeis commented Oct 26, 2023 • edited Loading

bonomat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bonomat commented Oct 26, 2023

luckysori left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holzeis commented Oct 26, 2023 •

edited

Loading